Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow a single notify-zulip notification to send multiple Zulip messages #1791

Merged

Conversation

fmease
Copy link
Member

@fmease fmease commented Apr 15, 2024

This would unblock rust-lang/rust#116957:

Hard Requirements:

We would like to

  1. ping the rustdoc team (via @*T-rustdoc*),
  2. link to the nominated PR (via the short link #NNN),
  3. create a Zulip poll.

Problems:

  • Unfortunately, neither pings nor short links inside the “question” (title) of a poll get recognized by Zulip.
    Example: Here, you can see that neither @*T-rustdoc* nor #123905 got autolinked.
  • Furthermore, it's not possible to sidestep the issue above by moving the ping and the PR ref “out of” the poll and into a separate — preceding — paragraph. If we tried that, Zulip would not detect the poll. This seems to be a known limitation: Create a poll | Zulip help center § Troubleshooting [archive link] (the live site no longer mentions this explicitly, only implicitly).

Solution:

Send two separate Zulip messages. This requires modifications to triagebot.


This PR enables users to specify a sequence of strings for the fields notify-zulip.⟨label⟩.message_on_⟨add|remove|close|reopen⟩ in order to make triagebot post a sequence of Zulip messages. For backward compatibility, you can still specify a single string. I haven't renamed any of the fields (like message_on_$ to messages_on_$) to avoid churn. A bare string gets treated as a sequence of length 1 containing that very string.

In TypeScript syntax, I've relaxed the type of the optional (!) fields message_on_$ from string | null to string | string[] | null.


r? @Mark-Simulacrum or infra

@fmease fmease force-pushed the notify-zulip-multiple-messages branch from c934d46 to c4ac8c3 Compare April 15, 2024 18:00
)]
pub(crate) messages_on_close: Vec<String>,
#[serde(
rename = "message_on_reopen",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should all the renames be alias? I guess it doesn't matter for just Deserialize?

Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine to me, though I have not yet reviewed in detail.

@Mark-Simulacrum Mark-Simulacrum merged commit f1c2168 into rust-lang:master Apr 16, 2024
2 checks passed
@fmease fmease deleted the notify-zulip-multiple-messages branch April 16, 2024 07:22
RalfJung added a commit to RalfJung/rust that referenced this pull request Apr 17, 2024
…n-backport-nominations, r=GuillaumeGomez

meta: notify #t-rustdoc Zulip stream on backport nominations

In July '23, it was decided to handle rustdoc-specific backport nominations in t-rustdoc meetings going forward ([Zulip announcement](https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/T-rustdoc.20backports/near/374828518)). However, t-rustdoc meetings are far too infrequent for them to address nominations on time (contrary to the weekly t-compiler meetings).

Hence GuillaumeGomez and I came to the conclusion that {beta,stable}-nominated rustdoc PRs should be dealt with on a case by case basis, e.g. on Zulip.

This PR attempts to partially automate this process. ~~Sadly, `triagebot` is not quite as flexible has I've hoped. Blocked on `triagebot` improvements (see the `FIXME`s in this PR).~~ (Fixed in rust-lang/triagebot#1791).

r? GuillaumeGomez
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 17, 2024
…n-backport-nominations, r=GuillaumeGomez

meta: notify #t-rustdoc Zulip stream on backport nominations

In July '23, it was decided to handle rustdoc-specific backport nominations in t-rustdoc meetings going forward ([Zulip announcement](https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/T-rustdoc.20backports/near/374828518)). However, t-rustdoc meetings are far too infrequent for them to address nominations on time (contrary to the weekly t-compiler meetings).

Hence GuillaumeGomez and I came to the conclusion that {beta,stable}-nominated rustdoc PRs should be dealt with on a case by case basis, e.g. on Zulip.

This PR attempts to partially automate this process. ~~Sadly, `triagebot` is not quite as flexible has I've hoped. Blocked on `triagebot` improvements (see the `FIXME`s in this PR).~~ (Fixed in rust-lang/triagebot#1791).

r? GuillaumeGomez
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 17, 2024
Rollup merge of rust-lang#116957 - fmease:meta-notify-rustdoc-zulip-on-backport-nominations, r=GuillaumeGomez

meta: notify #t-rustdoc Zulip stream on backport nominations

In July '23, it was decided to handle rustdoc-specific backport nominations in t-rustdoc meetings going forward ([Zulip announcement](https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/T-rustdoc.20backports/near/374828518)). However, t-rustdoc meetings are far too infrequent for them to address nominations on time (contrary to the weekly t-compiler meetings).

Hence GuillaumeGomez and I came to the conclusion that {beta,stable}-nominated rustdoc PRs should be dealt with on a case by case basis, e.g. on Zulip.

This PR attempts to partially automate this process. ~~Sadly, `triagebot` is not quite as flexible has I've hoped. Blocked on `triagebot` improvements (see the `FIXME`s in this PR).~~ (Fixed in rust-lang/triagebot#1791).

r? GuillaumeGomez
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Apr 17, 2024
…t-nominations, r=GuillaumeGomez

meta: notify #t-rustdoc Zulip stream on backport nominations

In July '23, it was decided to handle rustdoc-specific backport nominations in t-rustdoc meetings going forward ([Zulip announcement](https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/T-rustdoc.20backports/near/374828518)). However, t-rustdoc meetings are far too infrequent for them to address nominations on time (contrary to the weekly t-compiler meetings).

Hence GuillaumeGomez and I came to the conclusion that {beta,stable}-nominated rustdoc PRs should be dealt with on a case by case basis, e.g. on Zulip.

This PR attempts to partially automate this process. ~~Sadly, `triagebot` is not quite as flexible has I've hoped. Blocked on `triagebot` improvements (see the `FIXME`s in this PR).~~ (Fixed in rust-lang/triagebot#1791).

r? GuillaumeGomez
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants